How do you think about setHasSendWallets when there are multiple devices without device sync? I'm having trouble reasoning through it.

Notifications are now aware of if a stacker has send wallets via a new column users.sendWallets. I didn't simply pass this information as a GraphQL variable because notifications are rendered on the server and I didn't want to mess with that. Searching for getGetServerSideProps looked like we never pass GraphQL variables to it.¹

With this information, I simplified the notification logic such that filter for failed payment notifications is exactly the inverse of the filter for when automated retries should be attempted. To achieve this, I also reverted my decision to not show failed payments with userCancel set. They will not get automatically retried and thus should show up in notifications.

This means that if someone enables a send wallet, some notifications might disappear if they will now get automatically retried and vice versa. I think this is not bad UX but actually good.

edit: just saw your reply

How do you think about setHasSendWallets when there are multiple devices without device sync? I'm having trouble reasoning through it.

oof, you are right, a device with no send wallets would overwrite that information 🙄

Additionally, user.sendWallets is nice information for us.

I agree.

Anyway, to add some brainstorming. In a situation where we're worried about invoice "waste," don't want to retry without send wallets, and I'm thinking about the refactor, I think we should just send hasSendWallet with paid actions. We already do it with zaps.

We can probably write a graphql interface for paid action input types that requires hasSendWallet. retryPaidAction can also take it so that retries can be auto-retried if a wallet is added/found later.

dea2e7a now always retries, even without send wallets, to make sure that failed payments eventually show up in notifications.

However, there was an issue if a stacker goes offline longer than we would try automated retries. In that case, the payment counter will not reach the maximum and we need to check cancelledAt to show the notification.

This works to show the notification in /notifications, but not for the indicator, since the indicator assumes that updatedAt will get updated but it doesn't since the invoice isn't updated in a query.

To fix this, I added a retryTimeout job in ddc115e. I will sleep over this to make sure updating the invoice like this doesn't mess with anything.

You can test this case with the following patch which does the following thing:

• faster notification indicator
• do not set userCancel for QR code cancel
• faster retry timeout
• don't poll for retries

diff --git a/components/use-has-new-notes.js b/components/use-has-new-notes.js
index dcc843f3..7d489056 100644
--- a/components/use-has-new-notes.js
+++ b/components/use-has-new-notes.js
@@ -11,7 +11,7 @@ export function HasNewNotesProvider ({ me, children }) {
     SSR
       ? {}
       : {
-          pollInterval: NORMAL_POLL_INTERVAL,
+          pollInterval: 1_000,
           nextFetchPolicy: 'cache-and-network',
           onCompleted: ({ hasNewNotes }) => {
             if (!hasNewNotes) {
diff --git a/components/use-qr-payment.js b/components/use-qr-payment.js
index dddc53e9..de766618 100644
--- a/components/use-qr-payment.js
+++ b/components/use-qr-payment.js
@@ -20,7 +20,7 @@ export default function useQrPayment () {
       let paid
       const cancelAndReject = async (onClose) => {
         if (!paid && cancelOnClose) {
-          const updatedInv = await invoice.cancel(inv, { userCancel: true })
+          const updatedInv = await invoice.cancel(inv, { userCancel: false })
           reject(new InvoiceCanceledError(updatedInv))
         }
         resolve(inv)
diff --git a/lib/constants.js b/lib/constants.js
index 9ec20245..e35dc2f5 100644
--- a/lib/constants.js
+++ b/lib/constants.js
@@ -201,8 +201,8 @@ export const WALLET_CREATE_INVOICE_TIMEOUT_MS = 45_000
 // interval between which failed invoices are returned to a client for automated retries.
 // retry-after must be high enough such that intermediate failed invoices that will already
 // be retried by the client due to sender or receiver fallbacks are not returned to the client.
-export const WALLET_RETRY_AFTER_MS = 60_000 // 1 minute
-export const WALLET_RETRY_BEFORE_MS = 86_400_000 // 24 hours
+export const WALLET_RETRY_AFTER_MS = 5_000 // 1 minute
+export const WALLET_RETRY_BEFORE_MS = 10_000 // 24 hours
 // we want to attempt a payment three times so we retry two times
 export const WALLET_MAX_RETRIES = 2
 // when a pending retry for an invoice should be considered expired and can be attempted again
diff --git a/wallets/index.js b/wallets/index.js
index d72227b5..94b8043d 100644
--- a/wallets/index.js
+++ b/wallets/index.js
@@ -254,6 +254,7 @@ function RetryHandler ({ children }) {
   useEffect(() => {
     // we always retry failed invoices, even if the user has no wallets on any client
     // to make sure that failed payments will always show up in notifications eventually
+    return
 
     const retryPoll = async () => {
       let failedInvoices

This works to show the notification in /notifications, but not for the indicator, since the indicator assumes that updatedAt will get updated but it doesn't since the invoice isn't updated in a query.

To fix this, I added a retryTimeout job in ddc115e. I will sleep over this to make sure updating the invoice like this doesn't mess with anything.

I'm surprised the indicator query couldn't be changed instead.

Since possibly receiving a notification about a failed payment after 1 day is bad UX, I changed it to stop automated retries after one hour.

I'm surprised the indicator query couldn't be changed instead.

I think this is the first case where we want to show something in notifications after a timeout. This means it already existed in the database and wasn't updated between last poll and when we want to show it.

I considered to calculate updated_at on the fly using a CASE statement but I suspect that would mess with index usage.

And we can't create an index with an expression that depends on the current time afaik 🤔

So not sure how to change it. Using a job to update the timestamp makes everything work as expected.

However, we could add a new final FAILED state. That would also be an async job though.

Oh, I am still missing the fix for sortTime

Probably not much better than your job

btw, I think your suggestion is much better than my job